Pagination Cats fetching fixed #81
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe pagination and filtering logic for displaying CATs was refactored to use a cached, filtered list for more efficient state management. Functions were reorganized to separate filtering and pagination, with new state variables and updated UI handling. The sync URL parameter and related event-driven sync logic were removed. Additionally, a minor text change was made to the minter role description in the dropdown component and deployment redirect logic was simplified. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MyCATsPage
participant Storage
User->>MyCATsPage: Loads page or applies filter/search
MyCATsPage->>Storage: Fetch all CATs matching filter/search
Storage-->>MyCATsPage: Return filtered CATs
MyCATsPage->>MyCATsPage: Cache filtered CATs (allFilteredCATs)
MyCATsPage->>MyCATsPage: Slice current page CATs (currentPageCATs)
User->>MyCATsPage: Navigates to a different page
MyCATsPage->>MyCATsPage: Slice new page from cached allFilteredCATs
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
web/src/app/my-cats/page.tsx (1)
419-425: Avoid double slicing per page change
goToPagecallsupdateCurrentPageCATs(page)immediately, and theuseEffecttied topagination.currentPagetriggers the same slice right after → unnecessary work.Option A (simplest): drop the direct call and rely solely on the effect.
- setPagination(prev => ({ ...prev, currentPage: page })); - updateCurrentPageCATs(page); + setPagination(prev => ({ ...prev, currentPage: page }));Option B: keep direct slice and remove the effect listener.
Either way keeps render output identical while cutting duplicate computation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/src/app/my-cats/page.tsx(6 hunks)web/src/components/CatRoleDropdown.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
web/src/app/my-cats/page.tsx (1)
web/src/utils/indexedDB.ts (1)
CatDetails(6-15)
🔇 Additional comments (1)
web/src/components/CatRoleDropdown.tsx (1)
33-36: Change looks goodText tweak improves clarity without touching logic.
| const creatorCount = filteredCATs.filter(cat => cat.userRole === 'admin').length; | ||
| const minterCount = filteredCATs.filter(cat => cat.userRole === 'minter').length; | ||
| const totalPages = Math.ceil(totalCATs / pagination.catsPerPage); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Role counts ignore "both" and may double-count duplicates
Filtering by cat.userRole === 'admin' / 'minter' excludes 'both'.
If the same CAT is stored twice (one per role), counts/skipped duplicates skew metrics & pagination.
Suggestion:
-const creatorCount = filteredCATs.filter(c => c.userRole === 'admin' || c.userRole === 'both').length;
-const minterCount = filteredCATs.filter(c => c.userRole === 'minter' || c.userRole === 'both').length;
-const uniqueCats = new Map(filteredCATs.map(c => [c.address, c])).values();
+const uniq = Array.from(new Map(filteredCATs.map(c => [c.address, c])).values());then use uniq for totals/pagination to avoid duplicates.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In web/src/app/my-cats/page.tsx around lines 389 to 391, the current role counts
only check for 'admin' or 'minter' and ignore the 'both' role, which causes
inaccurate counts and potential double-counting if the same CAT appears twice.
Update the filters to include cats with userRole 'both' when counting admins and
minters. Additionally, use a method like uniq to remove duplicate CATs before
calculating total counts and pagination to ensure metrics are accurate and
duplicates do not skew results.
Summary by CodeRabbit
Refactor
Style
Documentation